-
Notifications
You must be signed in to change notification settings - Fork 24
[PM-26542] Add listing items functionality with session-based authentication with environment variable support for API key login #492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…turn session token to reuse
|
Thank you for your contribution! We've added this to our internal tracking system for review. Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #492 +/- ##
==========================================
- Coverage 78.10% 77.57% -0.54%
==========================================
Files 283 284 +1
Lines 27628 27818 +190
==========================================
Hits 21579 21579
- Misses 6049 6239 +190 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Apply cargo fmt to fix import ordering and line wrapping to comply with CI style checks.
Separate external crate imports from internal crate imports with blank lines to comply with nightly rustfmt group_imports feature used in CI.
addisonbeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Will re-approve if you decide to rename text_or_env_prompt, but that's not blocking.
|
Great job! No new security vulnerabilities introduced in this pull request |
|
nick-livefront
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if there is future work slated to flush out the implementation of the vault related items. If list is just being used as an easy implementation to test against, the changes are good with me.
Add unimplemented archive feature to match Node.js CLI: - Add `archive` command with todo!() placeholder - Add `--archived` flag to list command - Update ListOptions struct with archived field Should align with bitwarden/clients#16502 for future implementation.
nick-livefront
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing my feedback!
Please don't hesitate to propose more changes if needed, happy to help! |
Hinton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add this into the core crate? I was imaging we could temporarily add some glue code to the cli expecially since this implementation isn't really compliant to the current cli.
@Hinton I do believe it probably should be somewhere else, and agreed that the direct key extraction/injection might violate KM architecture. Please don't hesitate to point me out on how to refactor this situation. My current idea is to move export/import to |
|
Not sure where you got I hacked together bitwarden-labs#1 which ilustrates how we can implement this in the least invasive way possible to other crates. Additionally after discussions with other people we don't feel comfortable calling this session, because it works in a fundamentally different way from session in the regular cli. Rather than providing an encryption key the session is just a b64 encoded dump of very sensitive data. To that end, we think renaming |




🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-26542
📔 Objective
Implements session-based authentication for the Rust Bitwarden CLI, enabling stateless command execution similar to the Node.js CLI. Users can now authenticate once with
bw login api-keyand reuse the session for subsequent commands.Added environment variable support (
BW_CLIENTID,BW_CLIENTSECRET,BW_PASSWORD) to thebw login api-keycommand and made it return a base64-encoded session key (like the Node.js CLI, although this one is reaally long) that can be potentially used with --session (orBW_SESSION) for subsequent vault operations.Session Management
export_session()to serialize full client stateimport_session()to restore complete authenticated state from session stringEnvironment Variable Support
Added support for environment variables in
bw login api-key:BW_CLIENTID(orBW_CLIENT_ID): API client IDBW_CLIENTSECRET(orBW_CLIENT_SECRET): API client secretBW_PASSWORD: Master passwordList Command Implementation
bw list itemswith filtering options:--search: Text search across items--folderid: Filter by folder--collectionid: Filter by collection--organizationid: Filter by organization--trash: Show deleted itemsSession Usage
The global
--sessionflag andBW_SESSIONenvironment variable now work across all commands, automatically restoring client state before execution.Usage Example
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes